- ❌ Post-processed charts can briefly issue unsafe sorted queries.
- ⚠️ Early header clicks may show incorrect cumulative/rolling values.
- ⚠️ Behavior contradicts documented intent to disable server sort.

1. Configure a chart whose query uses post-processing (e.g., a cumulative or rolling
operation) so that the backend query context includes a non-empty `post_processing` list
alongside `orderby`, as reflected in
`tests/unit_tests/common/test_query_context_processor.py:1086-1092` (showing `"orderby":
[("Net Amount In", False)], "post_processing": [...]` in the same query).

2. Open the Explore results data panel for that chart: `DataTablesPane` instantiates
`useResultsPane` (`DataTablesPane.tsx:185-198`), which initializes `hasPostProcessing` to
`false` (`useResultsPane.tsx:73`) and immediately kicks off two effects: one that fetches
results via `getChartDataRequest` (`useResultsPane.tsx:108-147`) and one that
asynchronously detects `post_processing` by calling `buildV1ChartDataPayload` and updating
`hasPostProcessing` based on `payload.queries[].post_processing`
(`useResultsPane.tsx:155-186`).

3. As soon as the first result set comes back, `isLoading` is set to false and
`resultResp` is populated (`useResultsPane.tsx:130-145`), causing the hook to return
`SingleQueryResultPane` instances (`useResultsPane.tsx:228-258`); until the
`buildV1ChartDataPayload` promise resolves and `setHasPostProcessing(true)` runs,
`hasPostProcessing` remains `false`.

4. In that window, because `!hasPostProcessing && result.rowcount >= effectiveRowLimit` is
true, `onServerSort={handleServerSort}` is passed down (`useResultsPane.tsx:247-253` →
`SingleQueryResultPane.tsx:95-105` → `GridTable/index.tsx:49-65`), so clicking a column
header sends a server-side `orderby` while `row_limit` is applied to the raw SQL feeding
post-processing, potentially truncating or reordering the input rows for
cumulative/rolling operations and yielding incorrect post-processed values, even though
the design comment in `useResultsPane.tsx:68-72` indicates post-processed queries should
never be server-sorted.

This is a comment left during a code review.

**Path:** superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
**Line:** 73:73
**Comment:**
	*Incorrect Condition Logic: Initializing post-processing detection to `false` temporarily enables server-side sorting before async detection finishes, which can send incorrect `orderby + row_limit` requests for post-processed queries. Start in a disabled/unknown state and enable server sort only after detection confirms no post-processing.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

- ⚠️ Results pane clear-sort reverts to metric default ordering.
- ⚠️ Grid header shows no sort while backend still orders.
- ⚠️ Users may misinterpret "unsorted" as raw row order.

1. Open the Explore view and ensure the results data panel is visible so that
`DataTablesPane` renders (entry component at
`superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx:80-90`).

2. With the "Results" tab active, `DataTablesPane` calls `useResultsPane`
(`DataTablesPane.tsx:185-198`), which builds `cappedFormData` without an `orderby` field
when `orderby` state is empty (`useResultsPane.tsx:85-92`).

3. Because no `orderby` is provided in `cappedFormData`, `getChartDataRequest`
(`useResultsPane.tsx:121-128`) sends form data without `orderby`, and the backend query
builders apply their metric-based default ordering as verified by
`tests/unit_tests/mcp_service/chart/test_chart_helpers.py:860-868` (default `orderby` from
first metric when omitted).

4. Click a grid column header to sort: `GridTable` translates the Ag-Grid sort state into
an `orderby` array and calls `onServerSort(orderby)` (`GridTable/index.tsx:57-64`), which
`SingleQueryResultPane` passes through (`SingleQueryResultPane.tsx:97-105`) to
`useResultsPane.handleServerSort` (`useResultsPane.tsx:104-106`), updating `orderby` so
the next request includes an explicit `orderby` (`useResultsPane.tsx:85-92`); clicking the
same header again to clear the sort causes `GridTable` to send an empty `[]`,
`handleServerSort` resets `orderby` to `[]`, `cappedFormData` drops the `orderby` field
entirely (`...(orderby.length > 0 && { orderby })` at `useResultsPane.tsx:91`), and the
subsequent server request again uses the backend's implicit metric-based ordering while
the grid's header shows no active sort.

This is a comment left during a code review.

**Path:** superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
**Line:** 91:91
**Comment:**
	*Logic Error: Omitting `orderby` when the sort model becomes empty makes a "clear sort" action indistinguishable from "no explicit sort provided". Downstream query builders then re-apply their metric-based default ordering, so the grid can show `aria-sort="none"` while rows are still server-sorted. Keep an explicit empty `orderby` (or track a "user sorted" flag) so clearing sort actually clears server ordering.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

- ❌ Results pane can display rows for an old sort.
- ⚠️ Grid sort indicators may not match underlying row subset.
- ⚠️ Fast header clicks can cause confusing result "flapping."

1. In Explore, open the results data panel so `DataTablesPane` renders and calls
`useResultsPane` (`DataTablesPane.tsx:80-90` and `185-198`), and ensure the result set is
large enough that `result.rowcount >= effectiveRowLimit` so `onServerSort` is wired
(`useResultsPane.tsx:82-84` and `232-255`).

2. With a non–post-processed chart (so `hasPostProcessing` is false),
`SingleQueryResultPane` passes the `onServerSort` callback from `useResultsPane` down to
`GridTable` (`SingleQueryResultPane.tsx:49-65` and `95-105`), where Ag-Grid's
`onSortChanged` builds an `orderby` array from the current column sort state and invokes
`onServerSort(orderby)` (`GridTable/index.tsx:49-65`).

3. Rapidly click two different column headers (or repeatedly toggle sort) so that
`GridTable` fires multiple `onSortChanged` events in quick succession; for each event,
`handleServerSort` in `useResultsPane` updates `orderby` (`useResultsPane.tsx:104-106`),
which changes `cappedFormData` (`useResultsPane.tsx:85-93`) and triggers the `useEffect`
that starts a new `getChartDataRequest` without cancelling the previous one
(`useResultsPane.tsx:108-147`).

4. If the earlier request's network response arrives after the later one (e.g., the "old"
sort query is slower), its `.then` handler still calls `setResultResp(...)` and
`cache.set(cappedFormData, json.result)` (`useResultsPane.tsx:130-134`), and its
`.finally` sets `isLoading` false (`useResultsPane.tsx:143-145`), overwriting the more
recent sort's data because there is no AbortController or request-id check here (unlike
the guarded, abortable flow in `exploreJSON` at `chartAction.ts:221-249` and the
stale-response dropping at `chartAction.ts:278-79`), leaving the grid showing sort UI for
the last header clicked but row data corresponding to an earlier `orderby`.

This is a comment left during a code review.

**Path:** superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
**Line:** 104:106
**Comment:**
	*Race Condition: The new server-sort path can fire multiple overlapping requests (e.g., rapid header clicks), but there is no request sequencing or cancellation, so a slower older response can overwrite a newer sort result. Add a request id/AbortController guard and only apply the latest response to prevent stale data races.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

- ❌ Table chart server-side sorting ignores runtime mode changes.
- ⚠️ Explore users see inconsistent sorting after toggling pagination.

1. In `superset-frontend/plugins/plugin-chart-table/src/transformProps.ts` lines 521–523,
the table chart's `server_pagination` control from `rawFormData` is mapped into the
boolean `serverPagination` prop returned from `transformProps` and passed into
`TableChart` via `TableChartTransformedProps` (see `src/types.ts` lines 5–13 where
`serverPagination: boolean; serverPaginationData; setDataMask` are defined).

2. In `superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx` lines 392–52,
`TableChart` destructures `serverPagination = false`, `serverPaginationData`, and
`setDataMask` from `props`, and later defines `handleSortByChange` at lines 54–66 (from
the `Read` output) as:

   - `if (!serverPagination) return;`

   - build `modifiedOwnState` from `serverPaginationData` with the new `sortBy` and
   `currentPage: 0`

   - call `updateTableOwnState(setDataMask, modifiedOwnState)`

   This callback is memoized with `useCallback` and a dependency array that currently only
   includes `[serverPaginationData, setDataMask]` (line 1545 in the diff).

3. When a user toggles the "Server pagination" control for a table chart in Explore,
`transformProps` recomputes props with `serverPagination` flipped from `false` to `true`
(or vice versa) while keeping the same `serverPaginationData` object and `setDataMask`
hook reference (see `transformProps.ts` lines 36–43 and 39–41 where `serverPagination`
controls whether `serverPaginationData` comes from ownState or defaults). Because
`useCallback` in `TableChart.tsx` depends only on `[serverPaginationData, setDataMask]`,
React reuses the previous `handleSortByChange` function instance, whose closure still
captures the old `serverPagination` boolean.

4. After this toggle, when the user clicks a column header to sort, the `DataTable`
component in `superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx`
lines 142–162 detects a sort change (`sortBy`) and, if `serverPagination` is true, calls
`handleSortByChange`:

   - `const serverSortBy = serverPaginationData?.sortBy || [];`

   - `if (serverPagination && !isEqual(sortBy, serverSortBy)) { ...
   handleSortByChange([...]); }`

   At this point `DataTable` sees `serverPagination === true` (new prop) and invokes
   `handleSortByChange`, but inside that callback the captured `serverPagination` is still
   `false`, so it immediately returns and never calls `updateTableOwnState`. As a result,
   no server-side `sortBy` is pushed into `serverPaginationData`, the chart-data request
   is not reissued, and the "server-side column sorting" path described in this PR
   silently fails after toggling server pagination at runtime. Adding `serverPagination`
   to the `useCallback` dependency array forces `handleSortByChange` to be recreated when
   the mode changes, keeping the sort behavior in sync with the current pagination mode.

This is a comment left during a code review.

**Path:** superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
**Line:** 1545:1545
**Comment:**
	*Logic Error: The callback dependency list is incomplete: it reads `serverPagination` inside `handleSortByChange` but does not include it in the `useCallback` dependencies. If `serverPagination` flips (for example when result truncation state changes), this callback can keep a stale value and either skip required server re-sorts or keep issuing server sort updates when it should not. Add `serverPagination` to the dependency array so sorting behavior stays in sync with current mode.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

- ⚠️ Multi-query chart requests have inconsistent sort validation.
- ⚠️ Later queries can carry non-boolean sort directions.
- ⚠️ Downstream SQL builders see unexpected `orderby` value types.
- ⚠️ API contract "boolean sort direction" only partially enforced.

1. Note that the Chart Data endpoint `ChartDataRestApi.data` in
`superset/charts/data/api.py:93-200` accepts a query context JSON body where `"queries"`
is documented as "one or many query objects" (docstring lines 115–116).

2. Observe that this method computes `orderby` using only the first query: at
`superset/charts/data/api.py:152-161`, it assigns `orderby = (json_body.get("queries",
[{}])[0].get("orderby", []) if isinstance(json_body, dict) and json_body.get("queries")
else [])` and then calls `sort_error = validate_sort_params(orderby)`.

3. Note that the query context is built from *all* query objects:
`QueryContextFactory.create` in `superset/common/query_context_factory.py:45-89` iterates
over every element in the incoming `queries` list and passes each `query_obj` dict
(including its `orderby`) into `QueryObjectFactory.create`, producing a list of
`QueryObject` instances assigned to `QueryContext.queries`.

4. Craft a POST request to the `api/v1/chart/data` endpoint (which is wired to
`ChartDataRestApi.data` via `ChartRestApi` in `superset/charts/api.py`) with a JSON body
like:

   `{"datasource": {...}, "queries": [{"orderby": [("metric1", true)]}, {"orderby":
   [("metric2", "asc")]}, ...], "result_type": "full", "form_data": {...}}`. The first
   query's `orderby` uses a valid boolean direction, while the second query uses the
   invalid string `"asc"`.

5. When this request reaches `ChartDataRestApi.data`, the first query's `orderby` list is
extracted and passed to `validate_sort_params`, which returns `None` because all
directions in that list are booleans (see
`superset/tests/unit_tests/charts/data/test_api.py:13-20` where valid boolean directions
are accepted). No validation is ever performed on the second query's `orderby` list, so
the non-boolean `"asc"` is never rejected.

6. The method then constructs a `ChartDataCommand` with a `QueryContext` that includes
*all* queries, including the second query whose `orderby` contains a non-boolean
direction. Since `QueryObject` in `superset/common/query_object.py:112-152` simply assigns
`self.orderby = orderby or []` without type-checking, this malformed sort directive is
accepted and propagated, meaning later queries bypass the sort-direction validation that
the new helper `validate_sort_params` was meant to enforce.

7. This leads to inconsistent behavior: malformed sort directions in the first query
result in an immediate HTTP 400 from `validate_sort_params`, while the same malformed
directions in any subsequent query are silently accepted and only expressed (as
mis-sorting or backend errors) deeper in the query pipeline instead of being rejected
uniformly at the API boundary.

This is a comment left during a code review.

**Path:** superset/charts/data/api.py
**Line:** 397:401
**Comment:**
	*Incomplete Implementation: The new sort-direction validation only inspects `queries[0]`, so any additional query objects in the same request bypass this strict boolean check. This creates inconsistent behavior where malformed sort directions in later queries are silently accepted (or coerced) instead of being rejected. Validate `orderby` for every entry in `json_body["queries"]`, not just the first one.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix